Skip to content

Conversation

sosweetham
Copy link
Member

@sosweetham sosweetham commented Aug 19, 2025

Description of change

fixes some ui issues

Issue Number

Progresses 294 (does not close 100%)

Type of change

  • Fix (a change which fixes an issue)

How the change has been tested

Manual

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • New Features
    • QR scan permission prompts refined; login pages now include platform-specific eID App links, a 60‑second validity notice (“button” on mobile, “code” on desktop), and clickable W3DS logos.
  • Chores
    • Updated dependencies for Group Charter Manager; enabled network-accessible dev server for Pictique.
  • Style
    • Minor formatting/indentation adjustments.
  • Misc
    • Added temporary console logs for profile debugging.

Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Broadened camera permission handling in the eID Wallet scan page and added re-request logic; multiple login screens now link to platform-specific eID App store URLs and show 60-second validity notices; package manifests and config adjusted; small debug logs added to profile fetches.

Changes

Cohort / File(s) Change summary
eID Wallet: scan page
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte
Adjusted startScan permission handling to treat "denied" like "prompt" and re-request; preserved existing scan/auth and signing flow semantics (no exported API changes).
Login UI: app-store links & 60s notices
platforms/blabsy/src/components/login/login-main.tsx, platforms/eVoting/src/app/(auth)/login/page.tsx, platforms/group-charter-manager/src/components/auth/login-screen.tsx, platforms/pictique/src/routes/(auth)/auth/+page.svelte
Added local getAppStoreLink helpers; replaced plain "eID App" text with clickable store links; added device-aware “button/code valid for 60 seconds” notices; wrapped W3DS logo images with external link anchors; small JSX/ClassName fixes.
Group Charter Manager: config & deps
platforms/group-charter-manager/next.config.ts, platforms/group-charter-manager/package.json
next.config.ts formatting only; package.json reorganized and added dependencies (Next/React, class-variance-authority, clsx, react-hook-form, turndown, @next/env).
Pictique: dev script
platforms/pictique/package.json
Changed dev script to vite dev --host to enable network binding.
Pictique: auth/profile UI & logs
platforms/pictique/src/routes/(protected)/profile/[id]/+page.svelte, platforms/pictique/src/routes/(auth)/auth/+page.svelte
Added console.log debug statements for owner/fetched profile responses; imported additional PUBLIC_APP_STORE_EID_WALLET and PUBLIC_PLAY_STORE_EID_WALLET env vars and linked logo to external site; consolidated mobile-detection imports.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ScanPage
  participant Permissions
  participant Camera
  participant QRProcessor
  participant AuthHandler
  participant SigningHandler
  participant Redirect

  User->>ScanPage: Open scan page
  ScanPage->>Permissions: query/get camera permission
  alt permission granted
    ScanPage->>Camera: start scanning
    Camera-->>QRProcessor: QR payload
    alt payload starts with w3ds://sign
      QRProcessor->>SigningHandler: handle signing request
      SigningHandler->>Redirect: attempt redirect/post to redirect_uri
    else auth payload
      QRProcessor->>AuthHandler: handle auth
      AuthHandler->>Redirect: attempt redirect/assign/replace
    end
  else permission denied/prompt
    Permissions-->>ScanPage: "denied" treated like "prompt", re-request
    ScanPage-->>User: show permission UI or error if still denied
  end
  ScanPage->>Camera: cleanup on destroy
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • prototype#301 — Overlaps the same scan-QR permission handling change in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte.
  • prototype#277 — Modifies the same login UI (platforms/blabsy/src/components/login/login-main.tsx) and likely overlaps on store-link and QR/login text changes.
  • prototype#292 — Alters the eVoting login page (platforms/eVoting/src/app/(auth)/login/page.tsx) to add deep-link/store-link behavior similar to this change.

Suggested reviewers

  • sosweetham
  • pixel-punk-20

Poem

I hopped through code and camera beams,
Re-requested perms and scanned your dreams.
Sixty seconds, then the code will flee,
I link the app, click—you're home with me. 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 923de47 and 09e411f.

📒 Files selected for processing (1)
  • infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (1 hunks)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/final-ui-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sosweetham sosweetham marked this pull request as ready for review August 19, 2025 02:26
@sosweetham sosweetham requested a review from coodos as a code owner August 19, 2025 02:26
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (8)
platforms/pictique/package.json (1)

7-7: Exposing dev server on the LAN — confirm intent and consider explicit host.

vite dev --host will bind to all interfaces (0.0.0.0). This is great for testing on real devices, but it also exposes your dev server to the local network.

  • If this is intentional, consider making it explicit and adding a separate script to avoid surprises:
-    "dev": "vite dev --host",
+    "dev": "vite dev --host 0.0.0.0",
+    "dev:local": "vite dev"
platforms/pictique/src/routes/(protected)/profile/[id]/+page.svelte (1)

21-22: Avoid logging full profile payloads in production (possible PII leak).

These logs can expose user data in browser devtools. Gate them behind a dev check or remove before release.

-            console.log('Owner Profile:', response.data);
+            if (import.meta.env.DEV) console.log('Owner Profile:', response.data);
-            console.log('Fetched Profile:', response.data);
+            if (import.meta.env.DEV) console.log('Fetched Profile:', response.data);

Also applies to: 31-33

platforms/eVoting/src/app/(auth)/login/page.tsx (2)

69-79: Harden UA detection and use env-configurable store URLs; fix TS typing for MSStream check.

  • Use NEXT_PUBLIC_* envs for store links (keeps parity with other apps and avoids hard-coding).
  • Improve iOS detection for iPad on iOS 13+ (Mac UA) via navigator.maxTouchPoints heuristic.
  • Avoid TS error for window.MSStream by not using that legacy check.

Suggested diff:

-    const getAppStoreLink = () => {
-			const userAgent = navigator.userAgent || navigator.vendor || window.opera;
-			if (/android/i.test(userAgent)) {
-				return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
-			}
-			if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
-				return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667"
-			}
-			return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
-		};
+    const getAppStoreLink = () => {
+        const playUrl =
+            process.env.NEXT_PUBLIC_PLAY_STORE_EID_WALLET ??
+            "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
+        const appStoreUrl =
+            process.env.NEXT_PUBLIC_APP_STORE_EID_WALLET ??
+            "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667";
+        const ua = navigator.userAgent || navigator.vendor || (window as any).opera || "";
+        if (/android/i.test(ua)) return playUrl;
+        const isIOS =
+            /iPad|iPhone|iPod/.test(ua) ||
+            (/\bMacintosh\b/.test(ua) && navigator.maxTouchPoints > 1);
+        return isIOS ? appStoreUrl : playUrl;
+    };

If you prefer strict parity with other platforms in this PR, I can also wire this file to use the exact same env var names used elsewhere.


103-110: Open store links in a new tab and add rel for safety.

Aligns with the external W3DS link behavior below and avoids navigation away from the app.

-                                <a href={getAppStoreLink()}><span className="font-bold underline">eID App</span></a>
+                                <a href={getAppStoreLink()} target="_blank" rel="noopener noreferrer">
+                                    <span className="font-bold underline">eID App</span>
+                                </a>
-                                <a href={getAppStoreLink()}><span className="font-bold underline">eID App</span></a>
+                                <a href={getAppStoreLink()} target="_blank" rel="noopener noreferrer">
+                                    <span className="font-bold underline">eID App</span>
+                                </a>
platforms/blabsy/src/components/login/login-main.tsx (1)

90-93: Open external store links in a new tab and prevent tab-nabbing

Since these are external links, add target="_blank" plus rel="noopener noreferrer" for safety and UX.

-                                    Click the button to open your <a href={getAppStoreLink()}><b><u>eID App</u></b></a> to login
+                                    Click the button to open your <a href={getAppStoreLink()} target="_blank" rel="noopener noreferrer"><b><u>eID App</u></b></a> to login
-                                    Scan the QR code using your <a href={getAppStoreLink()}><b><u>eID App</u></b></a> to login
+                                    Scan the QR code using your <a href={getAppStoreLink()} target="_blank" rel="noopener noreferrer"><b><u>eID App</u></b></a> to login

Also applies to: 108-112

platforms/group-charter-manager/src/components/auth/login-screen.tsx (1)

73-83: Type-safe access to window props and consider centralizing store-link logic

window.MSStream and window.opera are non-standard and not typed; use safe checks to keep TS happy. Also, this helper appears in multiple apps—consider moving to a shared util to avoid drift.

-  const getAppStoreLink = () => {
-			const userAgent = navigator.userAgent || navigator.vendor || window.opera;
+  const getAppStoreLink = () => {
+			const userAgent = navigator.userAgent || (navigator as any).vendor || ((window as any).opera ?? '');
 			if (/android/i.test(userAgent)) {
 				return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
 			}
-			if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
+			if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) {
 				return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667"
 			}
 			return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
 		};
platforms/pictique/src/routes/(auth)/auth/+page.svelte (2)

23-37: Prefer env-configured store URLs to avoid hard-coded links

You already import PUBLIC_APP_STORE_EID_WALLET and PUBLIC_PLAY_STORE_EID_WALLET. Use them here for configurability and consistency across environments.

-    getAppStoreLink = () => {
-      const userAgent = navigator.userAgent || navigator.vendor || window.opera;
-      if (/android/i.test(userAgent)) {
-        return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet';
-      }
-      if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
-        return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667';
-      }
-      return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet';
-    };
+    getAppStoreLink = () => {
+      const ua = (navigator.userAgent || (navigator as any).vendor || ((window as any).opera ?? '')).toString();
+      if (/android/i.test(ua)) return PUBLIC_PLAY_STORE_EID_WALLET;
+      if (/iPad|iPhone|iPod/.test(ua) && !('MSStream' in window)) return PUBLIC_APP_STORE_EID_WALLET;
+      return PUBLIC_PLAY_STORE_EID_WALLET;
+    };

144-146: Add rel to external link to prevent tab-nabbing

Include rel="noopener noreferrer" alongside target="_blank".

-	<a href="https://metastate.foundation" target="_blank">
+	<a href="https://metastate.foundation" target="_blank" rel="noopener noreferrer">
 		<W3dslogo />
 	</a>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 63647bf and 923de47.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (1 hunks)
  • platforms/blabsy/src/components/login/login-main.tsx (3 hunks)
  • platforms/eVoting/src/app/(auth)/login/page.tsx (3 hunks)
  • platforms/group-charter-manager/next.config.ts (1 hunks)
  • platforms/group-charter-manager/package.json (2 hunks)
  • platforms/group-charter-manager/src/components/auth/login-screen.tsx (4 hunks)
  • platforms/pictique/package.json (1 hunks)
  • platforms/pictique/src/routes/(auth)/auth/+page.svelte (5 hunks)
  • platforms/pictique/src/routes/(protected)/profile/[id]/+page.svelte (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
platforms/group-charter-manager/src/components/auth/login-screen.tsx (1)
platforms/group-charter-manager/src/lib/utils/mobile-detection.ts (1)
  • isMobileDevice (1-6)
platforms/blabsy/src/components/login/login-main.tsx (2)
platforms/blabsy/src/lib/utils/mobile-detection.ts (1)
  • getDeepLinkUrl (11-13)
platforms/eVoting/src/lib/utils/mobile-detection.ts (1)
  • getDeepLinkUrl (8-10)
🪛 GitHub Actions: Check Lint
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte

[error] 45-45: lint/suspicious/noExplicitAny: Unexpected any. Specify a different type. In +page.svelte:45. Command failed: 'npx @biomejs/biome lint ./src' (eid-wallet check-lint).

🔇 Additional comments (8)
platforms/group-charter-manager/next.config.ts (1)

4-4: LGTM — formatting-only change.

No behavioral impact; safe to merge.

platforms/group-charter-manager/package.json (1)

18-18: Mandatory: Replace [email protected] with a Tailwind v4-compatible solution

The legacy JS-based [email protected] is incompatible with Tailwind CSS v4’s CSS-first architecture. To avoid build-time errors and ensure animation utilities work correctly, please:

  • platforms/group-charter-manager/package.json
    • Remove the tailwindcss-animate dependency.
    • Add a v4-compatible package (e.g., tw-animate-css or tailwind-animate):
      npm uninstall tailwindcss-animate
      npm install tw-animate-css --save
  • tailwind.config.js (or wherever plugins are declared)
    • Remove or comment out require('tailwindcss-animate') from the plugins array.
  • Your global CSS (e.g., globals.css or styles.css)
    • After your @import "tailwindcss";, add:
      @import "tw-animate-css";
    • Remove the old @tailwindcss-animate plugin invocation if present.

Once updated, verify your animations render as expected. If you’d like a minimal diff or additional guidance for your specific template (shadcn, plain Tailwind, etc.), let me know.

⛔ Skipped due to learnings
Learnt from: pixel-punk-20
PR: MetaState-Prototype-Project/prototype#121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional directive system has been removed. The `tailwind base` directive should be replaced with `import "tailwindcss/preflight";` for just base styles, or `import "tailwindcss";` to include all layers (preflight, components, and utilities).
Learnt from: pixel-punk-20
PR: MetaState-Prototype-Project/prototype#121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional `tailwind base` directive is no longer available and should be replaced with `import "tailwindcss/preflight"` instead. The directive system has been changed in v4 to use direct imports.
platforms/eVoting/src/app/(auth)/login/page.tsx (1)

170-173: LGTM — external logo link uses target+rel correctly.

The external link wrapping W3DS logo is safely configured.

platforms/blabsy/src/components/login/login-main.tsx (1)

142-149: Clickable W3DS logo is a nice UX touch

Wrapping the logo with a secure external link (target="_blank" and rel="noopener noreferrer") is solid.

platforms/group-charter-manager/src/components/auth/login-screen.tsx (2)

87-87: LGTM on the spinner element tweak

Self-closing the spinner div is fine and reads cleaner.


171-179: Linking the W3DS logo externally with proper rel/target is correct

Good use of rel="noopener noreferrer" with target="_blank".

platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)

129-133: Dynamic 60s validity text reads well

Clean conditional text for mobile vs desktop.

infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (1)

49-86: Good: permission handling and scan lifecycle look solid

Guarding repeated scans with scanning, handling permission prompts, and using .finally to reset state is correct.

// Signing specific state
let signingSessionId = $state<string | null>(null);
let signingData = $state<any>(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix lint failure: replace any with a concrete type for signingData

The pipeline fails on an explicit any. Define a SigningRequest type and use it.

Apply this diff:

-    let signingData = $state<any>(null);
+    let signingData = $state<SigningRequest | null>(null);

Add these types (place near other type declarations):

type VoteData =
  | { optionId: string | number; ranks?: never; points?: never }
  | { optionId?: never; ranks: Record<string, number> ; points?: never }
  | { optionId?: never; ranks?: never; points: Record<string, number> };

interface SigningRequest {
  pollId: string;
  voteData: VoteData;
  userId?: string;
  redirect_uri?: string;
}
🧰 Tools
🪛 GitHub Actions: Check Lint

[error] 45-45: lint/suspicious/noExplicitAny: Unexpected any. Specify a different type. In +page.svelte:45. Command failed: 'npx @biomejs/biome lint ./src' (eid-wallet check-lint).

🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around line
45, replace the explicit any on signingData with the new SigningRequest type and
add the provided VoteData and SigningRequest type declarations near the other
type declarations in the file; specifically, add the three-part VoteData union
and SigningRequest interface (including pollId, voteData, optional userId and
redirect_uri) alongside existing types, then change the declaration `let
signingData = $state<any>(null);` to use the SigningRequest type so the linter
error for explicit any is resolved.

Comment on lines 93 to 107
try {
await axios.post(redirect, { ename: vault.ename, session });
codeScannedDrawerOpen = false;
// Check if this was from a deep link
let deepLinkData = sessionStorage.getItem("deepLinkData");
if (!deepLinkData) {
// Also check for pending deep link data
deepLinkData = sessionStorage.getItem("pendingDeepLink");
}
if (deepLinkData) {
try {
const data = JSON.parse(deepLinkData);
console.log(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate redirect before posting auth completion to third-party endpoint

Avoid posting to an invalid or unsafe URL. Bail out gracefully if the URL doesn’t pass validation.

-        try {
-            await axios.post(redirect, { ename: vault.ename, session });
+        try {
+            if (!isSafeHttpUrl(redirect)) {
+                console.error("Invalid or unsafe redirect URL; aborting auth POST", redirect);
+                codeScannedDrawerOpen = false;
+                return;
+            }
+            await axios.post(redirect, { ename: vault.ename, session });
             codeScannedDrawerOpen = false;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around lines
93 to 107, the code posts to the redirect endpoint without validating the URL
first; update the flow to validate the redirect before calling axios.post by
constructing a URL object from the redirect string (catching exceptions), ensure
the protocol is http/https, and enforce either a configured whitelist of allowed
hosts or confirm the redirect origin matches an expected trusted origin; if
validation fails, do not call axios.post, set codeScannedDrawerOpen=false as
appropriate, and log or surface a clear error and return early to bail out
gracefully.

Comment on lines 118 to 141
// Validate the redirect URL
if (
!data.redirect ||
typeof data.redirect !== "string"
) {
console.error(
"Invalid redirect URL:",
data.redirect,
);
loggedInDrawerOpen = true;
startScan();
return;
}
// Redirect back to the platform that initiated the request
try {
console.log("Attempting redirect to:", data.redirect);
// Check if URL is valid
try {
new URL(data.redirect);
console.log("Redirect URL is valid");
} catch (urlError) {
console.error("Invalid URL format:", urlError);
loggedInDrawerOpen = true;
startScan();
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Restrict deep-link redirect to http(s) before attempting navigation

The URL format check is good, but ensure the scheme is http(s) to avoid javascript: or other unwanted schemes.

-                        // Validate the redirect URL
-                        if (
-                            !data.redirect ||
-                            typeof data.redirect !== "string"
-                        ) {
+                        // Validate the redirect URL (presence and scheme)
+                        if (
+                            !data.redirect ||
+                            typeof data.redirect !== "string" ||
+                            !isSafeHttpUrl(data.redirect)
+                        ) {
                             console.error(
                                 "Invalid redirect URL:",
                                 data.redirect,
                             );
                             loggedInDrawerOpen = true;
                             startScan();
                             return;
                         }
-
-                        // Check if URL is valid
-                        try {
-                            new URL(data.redirect);
-                            console.log("Redirect URL is valid");
-                        } catch (urlError) {
-                            console.error("Invalid URL format:", urlError);
-                            loggedInDrawerOpen = true;
-                            startScan();
-                            return;
-                        }
+                        console.log("Redirect URL is valid and safe");

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around lines
118 to 141, after successfully constructing new URL(data.redirect) add an
explicit scheme check to only allow http and https (e.g. verify url.protocol ===
"http:" || url.protocol === "https:"), and if the protocol is not allowed, treat
it as invalid by logging an error, reopening the drawer, calling startScan(),
and returning; keep the existing try/catch structure and perform the protocol
check inside the try block right after new URL(...) so non-http(s) schemes like
javascript: are rejected before any navigation attempt.

Comment on lines 218 to 227
function handleAuthRequest(content: string) {
const url = new URL(content);
signingSessionId = url.searchParams.get("session");
const base64Data = url.searchParams.get("data");
const redirectUri = url.searchParams.get("redirect_uri");
platform = url.searchParams.get("platform");
const redirectUrl = new URL(url.searchParams.get("redirect") || "");
redirect = url.searchParams.get("redirect");
session = url.searchParams.get("session");
hostname = redirectUrl.hostname;
isSigningRequest = false;
codeScannedDrawerOpen = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden redirect parsing and validation to prevent unsafe URLs

new URL(url.searchParams.get('redirect') || '') can throw, and non-HTTP(S) schemes could slip through. Validate before using and show a friendly error/fallback.

-    function handleAuthRequest(content: string) {
-        const url = new URL(content);
-        platform = url.searchParams.get("platform");
-        const redirectUrl = new URL(url.searchParams.get("redirect") || "");
-        redirect = url.searchParams.get("redirect");
-        session = url.searchParams.get("session");
-        hostname = redirectUrl.hostname;
-        isSigningRequest = false;
-        codeScannedDrawerOpen = true;
-    }
+    function handleAuthRequest(content: string) {
+        const url = new URL(content);
+        platform = url.searchParams.get("platform");
+        const redirectParam = url.searchParams.get("redirect") || "";
+        session = url.searchParams.get("session");
+        // Validate redirect
+        if (!isSafeHttpUrl(redirectParam)) {
+            console.error("Invalid or unsafe redirect URL:", redirectParam);
+            codeScannedDrawerOpen = false;
+            return;
+        }
+        redirect = redirectParam;
+        const redirectUrl = new URL(redirectParam);
+        hostname = redirectUrl.hostname;
+        isSigningRequest = false;
+        codeScannedDrawerOpen = true;
+    }

Add this helper near the top-level script:

function isSafeHttpUrl(urlString: string): boolean {
  try {
    const u = new URL(urlString);
    return u.protocol === "http:" || u.protocol === "https:";
  } catch {
    return false;
  }
}
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around lines
218 to 227, the code calls new URL(url.searchParams.get("redirect") || "") which
can throw and allow non-http(s) schemes; add the suggested isSafeHttpUrl helper
at the top-level script, wrap parsing in a try/catch or guard with isSafeHttpUrl
before constructing a URL object, only set redirect and hostname when the
redirect is a safe http(s) URL, otherwise set redirect/hostname to null or a
safe fallback and surface a friendly error/message to the user; ensure
session/platform assignment and drawer open logic remain unchanged when redirect
is invalid.

Comment on lines 299 to 307
const response = await fetch(redirect, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(signedPayload),
});
if (!response.ok) {
throw new Error("Failed to submit signed payload");
}
if (!response.ok) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate redirect before sending signed payload

Same concern here; ensure redirect points to http(s) only.

-            const response = await fetch(redirect, {
+            if (!isSafeHttpUrl(redirect)) {
+                throw new Error("Invalid or unsafe redirect URI");
+            }
+            const response = await fetch(redirect, {
                 method: "POST",
                 headers: {
                     "Content-Type": "application/json",
                 },
                 body: JSON.stringify(signedPayload),
             });

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around lines
299-307, the code posts the signedPayload to redirect without validating the
redirect URL; validate that redirect is a well-formed URL and that its protocol
is only "http:" or "https:" before calling fetch. Implement a try/catch that
constructs a new URL(redirect) (or otherwise parses it), verify url.protocol ===
"http:" || url.protocol === "https:", and if the check fails return/log an error
and do not call fetch; proceed with the existing fetch only when the validation
passes.

Comment on lines +44 to +53
const getAppStoreLink = () => {
const userAgent = navigator.userAgent || navigator.vendor || window.opera;
if (/android/i.test(userAgent)) {
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
}
if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667"
}
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Guard getAppStoreLink for SSR to avoid ReferenceError

This component is not marked as a client component, so navigator/window access during SSR will throw. Add guards (or mark the file with "use client"). Guarding keeps SSR intact.

Apply this diff:

-    const getAppStoreLink = () => {
-			const userAgent = navigator.userAgent || navigator.vendor || window.opera;
-			if (/android/i.test(userAgent)) {
-				return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
-			}
-			if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
-				return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667"
-			}
-			return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
-		};
+    const getAppStoreLink = (): string => {
+      // SSR-safe guards
+      if (typeof window === 'undefined' || typeof navigator === 'undefined') {
+        return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
+      }
+      const ua = (navigator.userAgent || (navigator as any).vendor || ((window as any).opera ?? '')).toString();
+      if (/android/i.test(ua)) {
+        return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
+      }
+      if (/iPad|iPhone|iPod/.test(ua) && !('MSStream' in window)) {
+        return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667";
+      }
+      return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
+    };

Optionally, consider centralizing this helper in a shared util to avoid duplication across platforms.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const getAppStoreLink = () => {
const userAgent = navigator.userAgent || navigator.vendor || window.opera;
if (/android/i.test(userAgent)) {
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
}
if (/iPad|iPhone|iPod/.test(userAgent) && !window.MSStream) {
return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667"
}
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
};
const getAppStoreLink = (): string => {
// SSR-safe guards
if (typeof window === 'undefined' || typeof navigator === 'undefined') {
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
}
const ua = (navigator.userAgent || (navigator as any).vendor || ((window as any).opera ?? '')).toString();
if (/android/i.test(ua)) {
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
}
if (/iPad|iPhone|iPod/.test(ua) && !('MSStream' in window)) {
return "https://apps.apple.com/in/app/eid-for-w3ds/id6747748667";
}
return "https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet";
};
🤖 Prompt for AI Agents
In platforms/blabsy/src/components/login/login-main.tsx around lines 44 to 53,
the getAppStoreLink function directly accesses navigator/window which will throw
during SSR; either mark the component as a client component by adding "use
client" at the top of the file (if it must run on the client) or guard the
function for SSR by checking typeof window !== "undefined" and typeof navigator
!== "undefined" before using them and returning a safe default app-store URL
when those globals are not available; optionally move this helper to a shared
util to avoid duplication across platforms.

@coodos coodos merged commit 71d40f2 into main Aug 19, 2025
0 of 4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Aug 26, 2025
6 tasks
@coderabbitai coderabbitai bot mentioned this pull request Sep 10, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants